Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Normalize case of SVG elements and attributes in HTML, and preserve accessibility attributes #6593

Merged
merged 4 commits into from
Jul 17, 2021

Conversation

devongovett
Copy link
Member

Fixes #4675. Fixes #5004.

HTML attributes and element names are case insensitive, including foreign content such as SVG. However, SVGO does not support this and expects case sensitive names. This normalizes element and attribute names within embedded SVG before running htmlnano so it works as expected.

In addition, this disables some SVGO options that remove accessibility attributes such as ARIA and role, as well as the <title> and <desc> elements. These should be preserved on SVG that is embedded within HTML.

@height
Copy link

height bot commented Jul 14, 2021

Link Height tasks by mentioning a task ID in the pull request title or description, commit messages, or comments.

💡Tip: You can also use "Close T-X" to automatically close a task when the pull request is merged.

@parcel-benchmark
Copy link

parcel-benchmark commented Jul 14, 2021

Benchmark Results

Kitchen Sink 🚨

Timings

Description Time Difference
Cold FAILED -0.00ms
Cached FAILED -0.00ms

Cold Bundles

No bundles found, this is probably a failed build...

Cached Bundles

No bundles found, this is probably a failed build...

React HackerNews ✅

Timings

Description Time Difference
Cold 9.32s +118.00ms
Cached 484.00ms +11.00ms

Cold Bundles

No bundle changes detected.

Cached Bundles

Bundle Size Difference Time Difference
dist/logo.1e014c76.png 274.00b +0.00b 224.00ms +15.00ms ⚠️

AtlasKit Editor 🚨

Timings

Description Time Difference
Cold FAILED -0.00ms
Cached FAILED -0.00ms

Cold Bundles

No bundles found, this is probably a failed build...

Cached Bundles

No bundles found, this is probably a failed build...

Three.js ✅

Timings

Description Time Difference
Cold 6.90s -140.00ms
Cached 413.00ms -17.00ms

Cold Bundles

No bundle changes detected.

Cached Bundles

No bundle changes detected.

Click here to view a detailed benchmark overview.

@@ -2322,8 +2322,16 @@ describe('html', function() {
});

it('should work with bundle names that have colons in them', async function() {
// Windows paths cannot contain colons, so write the file here (in memory).
await overlayFS.mkdirp(path.join(__dirname, 'integration/url-colon'));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this fix the colon issue? I notice the benchmarks still fail due to a invalid path with colons and I notice it on other PRs as well. Is that just a bug we introduced somewhere and is now in V2 ? 😟

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it should but it seems the test is still failing on windows. I will look into it

@devongovett devongovett merged commit a8a5b70 into v2 Jul 17, 2021
@devongovett devongovett deleted the svg-bugs branch July 17, 2021 23:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants